-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cargo run supports --package argument #3691
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
@bors: r+ Looks great, thanks! |
📌 Commit a158958 has been approved by |
⌛ Testing commit a158958 with merge 31aa5eb... |
💔 Test failed - status-appveyor |
Hm, the test failure seems legitimate, but it only triggers on mvsc... Not sure what's going on there, will try to look closer tomorrow. |
tests/run.rs
Outdated
.file("d3/src/main.rs", "fn main() { println!(\"d2\"); }"); | ||
|
||
let cargo_process = || { | ||
let mut process_builder = p.cargo_process("run"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you update this to cargo
instead of cargo_process
?
Windows doesn't like blowing away target
right after we compile something. By default cargo_process
will delete the tree and recreate it, where cargo
just creates a process.
This'll then just need a p.build()
up front.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then perhaps cargo_process
should consume ProjectBuilder
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI: I've tried to add dynamic checks that we don't build project twice, and turns out that some tests do, and relay on that:
-
Line 1428 in 2c2e07f
fn deletion_causes_failure() { -
Line 2458 in 2c2e07f
fn explicit_color_config_is_propagated_to_rustc() {
and
Line 792 in 2c2e07f
fn cargo_default_env_metadata_env_var() { |
cargo clean
and uses .build
more than once.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah some of these are flaky sometimes but a default-on dynamic check would be useful!
@bors: r+ Nah this looks good to me! |
📌 Commit 261ae46 has been approved by |
cargo run supports --package argument closes #3529
☀️ Test successful - status-appveyor, status-travis |
Assert that we don't build test project twice. Discussed in #3691 (comment). I've modify the offending tests to be more explicit about recreating projects.
closes #3529